Skip to content

Conversation

@pi0
Copy link
Contributor

@pi0 pi0 commented Jan 31, 2025

Fixes #7993 (/cc @petebacondarwin)

This PR adds -C, --cwd global argument to the wrangler CLI to allow changing the current working directory before running any command.

This feature is used in most of CLI tools that depend on right working directory but different names. I have chosen -C, --cwd following with pnpm (-C) + yarn (--cwd), but open to ideas changing it to anything else.

This allows better DX of using wrangler commands from any working directory to another (where project is).

$ wrangler --help

GLOBAL FLAGS
  -c, --config   Path to Wrangler configuration file  [string]
  -C, --cwd      Run as if wrangler was started in <path> instead of the current working directory  [string]
  -e, --env      Environment to use for operations, and for selecting .env and .dev.vars files  [string]
  -h, --help     Show help  [boolean]
  -v, --version  Show version number  [boolean]

  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because: It is generic CLI behavior (is it needed?)
  • E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: effect of global arg has unit test
  • Public documentation

@changeset-bot
Copy link

changeset-bot bot commented Jan 31, 2025

🦋 Changeset detected

Latest commit: 5a2e268

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
wrangler Minor
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Jan 31, 2025

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-wrangler-7994

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7994/npm-package-wrangler-7994

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-wrangler-7994 dev path/to/script.js
Additional artifacts:

cloudflare-workers-bindings-extension:

wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-workers-bindings-extension-7994 -O ./cloudflare-workers-bindings-extension.0.0.0-v03fe04cf1.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-v03fe04cf1.vsix

create-cloudflare:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-create-cloudflare-7994 --no-auto-update

@cloudflare/kv-asset-handler:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-kv-asset-handler-7994

miniflare:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-miniflare-7994

@cloudflare/pages-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-pages-shared-7994

@cloudflare/unenv-preset:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-unenv-preset-7994

@cloudflare/vite-plugin:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-vite-plugin-7994

@cloudflare/vitest-pool-workers:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-vitest-pool-workers-7994

@cloudflare/workers-editor-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-workers-editor-shared-7994

@cloudflare/workers-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-workers-shared-7994

@cloudflare/workflows-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-workflows-shared-7994

Note that these links will no longer work once the GitHub Actions artifact expires.


[email protected] includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20250129.0
workerd 1.20250129.0 1.20250129.0
workerd --version 1.20250129.0 2025-01-29

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@pi0 pi0 changed the title feat(wrangler): support -C, --cwd cli argument feat(wrangler): support -C, --dir cli argument Jan 31, 2025
@pi0 pi0 changed the title feat(wrangler): support -C, --dir cli argument feat(wrangler): support -C, --cwd cli argument Jan 31, 2025
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and happy with the naming.

@pi0 pi0 marked this pull request as ready for review February 3, 2025 10:39
@pi0 pi0 requested review from a team as code owners February 3, 2025 10:39
@petebacondarwin
Copy link
Contributor

A minor linting issue to fix https://github.com/cloudflare/workers-sdk/actions/runs/13079905513/job/36575863616?pr=7994#step:5:916

@pi0
Copy link
Contributor Author

pi0 commented Feb 3, 2025

updated.

Copy link
Contributor

@penalosa penalosa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit uneasy with the -C shortening here—I know it's a relatively common convention, but we also have a global flag -c for config, and those seem pretty likely to be mixed up to me. Not a blocker though.

However, this really should have tests.

@pi0
Copy link
Contributor Author

pi0 commented Feb 3, 2025

Added basic test in packages/wrangler/src/__tests__/index.test.ts

Also made PR for docs: cloudflare/cloudflare-docs#19665

@pi0 pi0 force-pushed the feat/wrangler-cwd branch 2 times, most recently from 2f9e744 to 625faf0 Compare February 3, 2025 15:41
@pi0 pi0 force-pushed the feat/wrangler-cwd branch from 625faf0 to 5a2e268 Compare February 3, 2025 15:43
@pi0
Copy link
Contributor Author

pi0 commented Feb 4, 2025

@penalosa can you please review this? 🙏🏼

ip: "127.0.0.1",
inspectorPort: options?.inspectorPort ?? 0,
v: undefined,
cwd: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related directly to this PR... I wonder if we should actually change StartDevOptions to omit properties like this? Rather than having to pass in undefined?

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LGTM

@CarmenPopoviciu
Copy link
Contributor

I share @penalosa 's concern about -c and -C being confusing for users. I spoke with @petebacondarwin offline and he seemed fine with dropping -C altogether in favour of just --cwd. I'd like to propose we do that, unless there is other shortening that would make sense in this case. @pi0 any strong objections to that? I'm happy to help get this over the finish line if you don't have the bandwidth ♥️

@pi0
Copy link
Contributor Author

pi0 commented Feb 20, 2025

thanks for followup, dear @CarmenPopoviciu. -C is really standard in most of UNIX tools but of course totally fine to only keep --cwd (it was main requirement)

Do you mind to push change to this PR if have time? 🙏🏼

@CarmenPopoviciu
Copy link
Contributor

CarmenPopoviciu commented Feb 20, 2025

-C is really standard in most of UNIX tools

yeah, I noticed that being mentioned, so I was a bit torn on what the right thing to do is. Since changing the -c shortening would have much bigger impact, I figured dropping -C would be the most straightforward thing to do. Let's see where this change lands with users, and if there is an ask for -C we can revisit this decision and add it later.

Do you mind to push change to this PR if have time

totally happy to do that ♥️ . Don't think I can push directly to this PR since it's from a forked repo, so I might open a PR to your branch instead if that's alright?

@CarmenPopoviciu
Copy link
Contributor

CarmenPopoviciu commented Feb 20, 2025

@pi0 👉 pi0#1 ^.^. Oh and can u pls rebase on top of main again 🙏 ?

@CarmenPopoviciu
Copy link
Contributor

CarmenPopoviciu commented Feb 25, 2025

I've opened a new PR based off of this one, incl. the changes I suggested, so we can get this across the finish line.

@CarmenPopoviciu
Copy link
Contributor

Closing this in favour of #8253 ♥️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

🚀 Feature Request: Support argument for wrangler to run in different working directory

4 participants